Track the amount spent on fees as payments are retried #1142
Conversation
TheBlueMatt
force-pushed
the
2021-10-on-sent-fees
branch
from
f098572 to
017e251
Compare
Codecov Report
@@ Coverage Diff @@
## main #1142 +/- ##
==========================================
- Coverage 90.51% 90.51% -0.01%
==========================================
Files 69 69
Lines 35865 35891 +26
==========================================
+ Hits 32462 32485 +23
- Misses 3403 3406 +3
Continue to review full report at Codecov.
|
TheBlueMatt
force-pushed
the
2021-10-on-sent-fees
branch
from
017e251 to
d8f2725
Compare
| @@ -3464,21 +3484,22 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana | |||
| // restart. | |||
| // TODO: We should have a second monitor event that informs us of payments | |||
| // irrevocably fulfilled. | |||
| payment.get_mut().remove(&session_priv_bytes, Some(path.last().unwrap().fee_msat)); | |||
There was a problem hiding this comment.
I think the last two params here should be None for consistency.
I think it'd be a little more readable if we dropped the lock then called finalize_claims rather than duplicating the finalize_claims logic
There was a problem hiding this comment.
These actually should diverge more, see the TODO here - we'd really like to mark the payment fulfilled but not remove the HTLC until after the commitment signed dance completes.
| @@ -436,6 +436,8 @@ pub(crate) enum PendingOutboundPayment { | |||
| payment_hash: PaymentHash, | |||
| payment_secret: Option<PaymentSecret>, | |||
| pending_amt_msat: u64, | |||
| /// Used to track the fee paid. Only present if the payment was serialized on 0.0.103+. | |||
There was a problem hiding this comment.
suggested wording: "Used to track the fees from paths that have not yet succeeded or failed."
There was a problem hiding this comment.
Hmm? No, we use it to track the full fees, we just do that by only tracking the pending fees and then deciding that's the fees actually paid when we succeed, since it will be unless someone gave us free money.
TheBlueMatt
force-pushed
the
2021-10-on-sent-fees
branch
from
d8f2725 to
cd01677
Compare
|
Rebased on main and added the refactors requested, had to redo git history, though, so no fixup commits, sorry. |
TheBlueMatt
force-pushed
the
2021-10-on-sent-fees
branch
from
e385663 to
ce7da23
Compare
This will make the next commit much simpler
Especially once we merge the `InvoicePayer` logic soon, we'll want to expose the total fee paid in the `PaymentSent` event.
TheBlueMatt
force-pushed
the
2021-10-on-sent-fees
branch
from
ce7da23 to
04d4a8f
Compare
|
Squashed without diff, will land after CI: |
Especially once we merge the
InvoicePayerlogic soon, we'll wantto expose the total fee paid in the
PaymentSentevent.